Use the current Roller session during OAuth authorization - #165
Conversation
Derive the approving identity from the Roller session, as the rest of the UI does, and require the account to be enabled. Without a session the request goes to the login flow as before. A consumer key bound to a specific user may still only be approved by that user; a site-wide key is approved as whoever is logged in. Clients that continue to post the identity are accepted when the value agrees with the session and refused otherwise. Add OAuthManager.authorizeRequestToken(consumerKey, requestToken, userName), backed by a named update that matches the consumer key, the exact request token, an unauthorized record, and no access token, and reports whether one row changed. Approval is therefore one-shot, with no read-then-write window. markAsAuthorized is deprecated: it keyed on the consumer alone and did not name the token being approved. Refusals share one response so callers cannot tell refusals apart. Drop the identity field from the consent form and give it the standard salt field, and validate that token on the consent URL only. The request-token and access-token endpoints carry an OAuth signature and are left out of that mapping. Tests: AuthorizationServletTest. Claude-Session: https://claude.ai/code/session_01A1fhY1E2PCFU6UAPXu2WtV
There was a problem hiding this comment.
Reviewed. The session-binding, one-time approval, and validation changes are moving in the right direction. Please address the compatibility and error-handling notes before merge. Coordination with #154 remains necessary because both changes touch the same flow.
| q.setParameter(2, new Timestamp(new Date().getTime())); | ||
| q.setParameter(3, consumerKey); | ||
| q.setParameter(4, requestToken); | ||
| return q.executeUpdate() == 1; |
There was a problem hiding this comment.
markAsAuthorized was idempotent; this update requires authorized to be null or false, so approving a token that's already authorized but not yet exchanged affects zero rows and the servlet answers a bare 403 permission_denied instead of returning the user to the consumer. That happens on a retried or double-submitted approval. Treating already-authorized-by-the-same-user as success keeps the one-shot guarantee without breaking retries.
|
|
||
| @Override | ||
| public void doGet(HttpServletRequest request, HttpServletResponse response) | ||
| throws IOException, ServletException { |
There was a problem hiding this comment.
doGet still dereferences accessor without the null guard doPost gained (line 66): GET ...authorize?oauth_consumer_key= before any request token exists NPEs on accessor.getProperty and surfaces as a container 500, where doPost now returns the uniform permission_denied.
| returnToConsumer(request, response, accessor); | ||
|
|
||
|
|
||
| } catch (OAuthProblemException e) { |
There was a problem hiding this comment.
Previously OAuthProblemException from getAccessor (token_expired, token_rejected) went through OAuthServlet.handleException, which sends the problem-specific status and a WWW-Authenticate: OAuth realm header. This catch collapses everything to a bare 403 with no realm header, while doGet on the same endpoint still reports the old way, so the same token state is described two different ways depending on method.
| * Callers should not distinguish these cases to the client. | ||
| * @throws OAuthException on persistence failure | ||
| */ | ||
| boolean authorizeRequestToken(String consumerKey, String requestToken, String userName) |
There was a problem hiding this comment.
Minor: adding an abstract method to this interface while keeping markAsAuthorized deprecated 'for callers outside the project' is a bit contradictory; if external implementations are a concern, a default method covers them, and if they aren't, markAsAuthorized can just go.
2 participants
The OAuth 1.0a consent step should authorize the user who is signed in, the way
the rest of Roller's admin and editor UI resolves identity from the session.
This change moves it onto that model and tightens request-token approval into a
single conditional update.
What changed
user. A request with no session goes through the normal login flow.
userId/xoauth_requestor_idrequestparameter is still accepted when it agrees with the session user, and
rejected otherwise; it is not used to choose the identity.
exactly one row changed — rather than a separate load then store, so approval
is one-shot.
oauth_problem=permission_denied(403) for every refusal, sothe response does not vary with the reason.
markAsAuthorizedin favour ofauthorizeRequestToken.Tests
AuthorizationServletTestcovers identity taken from the session, a mismatcheduserIdparameter, disabled accounts, the bound-consumer refusal, and thegeneric refusal path.
JPAOAuthManagerTestexercises the conditional updateagainst Derby: a mismatched token changes no row, the exact pending token
succeeds once, and a second use of the same token changes no row.